Conversation
12cb36c to
fefef8a
Compare
There was a problem hiding this comment.
"Improve latency of lazy dynamic filters in some cases"? Or maybe just drop
There was a problem hiding this comment.
Or "Improve latency when dynamic filtering is in use" ?
There was a problem hiding this comment.
This is less about performance than about cost. We used to fetch 128 partitions per API call to Glue. We now fetch 1000. The query planning performance difference with and without patch was ~<5s (out of around 3m planning time, so... negligible).
This does lower cost and help avoid running into rate-limits. Not sure how that should be conveyed in release notes.
There was a problem hiding this comment.
@hashhar how about
* Decrease the number of requests to the Glue metastore when fetching partitions.
This helps avoid hitting rate limits and decreases service costs. ({issue}`4938`)There was a problem hiding this comment.
Although, not hitting a rate-limits does help performance.
losipiuk
left a comment
There was a problem hiding this comment.
I also added couple of comments to #4755
#4755 (comment)
#4755 (comment)
#4755 (comment)
#4755 (comment)
#4755 (comment)
kokosing
left a comment
There was a problem hiding this comment.
It looks like a huge release. A lot of items.
a84d805 to
37878f4
Compare
There was a problem hiding this comment.
is "fractional seconds" a valid term?
There was a problem hiding this comment.
Yes (vs "whole seconds"). What feels weird about it?
c13e42f to
20c6e75
Compare
There was a problem hiding this comment.
This is out of date with the new microsecond changes. Any tables containing timestamps need to be rewritten entirely. It's actually a different PR number, since the microsecond PR would cause such tables to be completely different.
There was a problem hiding this comment.
What's the suggestion? Remove it?
Closes #4755